Skip to content

Fix registry description overflow in settings table#2476

Closed
Philip-Carneiro wants to merge 1 commit intokubeflow:mainfrom
Philip-Carneiro:fix/registry-description-overflow
Closed

Fix registry description overflow in settings table#2476
Philip-Carneiro wants to merge 1 commit intokubeflow:mainfrom
Philip-Carneiro:fix/registry-description-overflow

Conversation

@Philip-Carneiro
Copy link
Copy Markdown
Contributor

@Philip-Carneiro Philip-Carneiro commented Mar 25, 2026 •
edited
Loading

Summary

  • Replace raw <p> tag with TruncatedText component (maxLines=2) from mod-arch-shared in ModelRegistriesTableRow
  • Prevents long registry descriptions from overflowing and breaking the table layout in Model Registry Settings
  • Follows the same pattern used in McpCatalogCard for description truncation

Test plan

  • Create a model registry with a very long description (200+ characters)
  • Verify the description is truncated to 2 lines in the settings table
  • Verify hovering/clicking shows the full description (TruncatedText built-in behavior)
  • Verify short descriptions still display normally without truncation
  • Verify existing unit tests pass (npm test -- --testPathPattern=settings)

@Philip-Carneiro @claude
Fix registry description overflow with TruncatedText component
80ef8ea 
Replace raw <p> tag with TruncatedText (maxLines=2) from mod-arch-shared
to prevent long registry descriptions from overflowing the table layout
in Model Registry Settings.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
@google-oss-prow google-oss-prow bot requested a review from chambridge March 25, 2026 14:56
@google-oss-prow
Copy link
Copy Markdown
Contributor

google-oss-prow bot commented Mar 25, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ederign for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot requested a review from fege March 25, 2026 14:56
@manaswinidas
Copy link
Copy Markdown
Contributor

manaswinidas commented Mar 25, 2026

/retest

@Philip-Carneiro
Copy link
Copy Markdown
Contributor Author

Philip-Carneiro commented Mar 25, 2026

CI Failure Analysis

The failing check E2E Py 3.11 K8s v1.33.7 DB db is unrelated to this PR's change (frontend-only: replacing a <p> tag with TruncatedText in ModelRegistriesTableRow.tsx).

Root cause

The failure occurs in ./scripts/utils.sh: line 29 where repeat_cmd_until uses [ "$output" = "$condition" ] — this breaks when kubectl returns multiple container image lines. The comparison expects a single value but receives multiple lines, causing [: too many arguments.

Evidence this is pre-existing/flaky

This PR's scope

This PR only modifies one file:

  • clients/ui/frontend/src/app/pages/settings/ModelRegistriesTableRow.tsx — replaces <p> with TruncatedText component

No backend, scripts, or deployment files were changed.

Could a maintainer please re-run the failed job? Thank you!

manaswinidas
manaswinidas reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@manaswinidas manaswinidas left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bug is not reproducible in mock mode in main so this PR should be closed IMO

Before and after it's the same:
Image

@manaswinidas
Copy link
Copy Markdown
Contributor

manaswinidas commented Mar 25, 2026

Ignore my previous comment - testing in settings page

manaswinidas
manaswinidas reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@manaswinidas manaswinidas left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The settings part is consumed midstream rather than from here - so we should close this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chambridge chambridge Awaiting requested review from chambridge

@fege fege Awaiting requested review from fege

1 more reviewer

@manaswinidas manaswinidas manaswinidas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area/UI size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Philip-Carneiro @manaswinidas